Fix infinite loop when using StreamingResponse with ASGI client #912
Conversation
JayH5
changed the title
JayH5
force-pushed
the
gh-908/stream-infinite-loop
branch
from
525dd4d
to
57db734
Compare
There was a problem hiding this comment.
Just got hit by this bug myself as soon as I plug a middleware built with BaseHTTPMiddleware into my middleware chain, as those middleware classes return StreamingResponse instances.
Tried this fix locally and it works like a charm. Arguably not the most elegant but it makes sense that we should give the event loop some air in this infinite loop.
|
Doesn't make any sense to me - we are already yielding control to the event loop |
|
@tomchristie as far as I understand, the If the @florimondmanca I'm curious about what client you were using? Also httpx? I'm not super-confident in this PR as it's not clear to me whether it should be the job of the client (httpx) to be doing this yielding in its ASGI backend (hard to do because of async abstractions there). But this is the best I have so far. |
|
@JayH5 Yes, HTTPX with the AsyncClient. @tomchristie As Jay said - the await doesn’t actually guarantee that the event loop does task switching somewhere. Trio has an explicit notion of “checkpoints” for this and I assume it wouldn’t be a problem there, but asyncio is more tricky to get right sometimes because of this. FWIW I remember we already had to resort to sleep(0) for some asyncio fixes in the past. We have one in HTTPX for a Python 3.6 compatibility issue somewhere that was exactly caused by this problem (asyncio not having the chance to do a round of its event loop). |
|
@JayH5 I agree that this might be more of a client / HTTPX concern. In normal conditions (eg when uvicorn is calling the app) the receive() call waits on a socket, which triggers event loop checkpoints. I assume HTTPX should emulate that and make sure that it doesn’t provide a secretly synchronous receive() function - eg because the body iterator actually draws its chunks from a list or similar. |
|
Fixed on the client side in encode/httpx#919 |
Fixes #908
Without this, the
run_until_first_completefunction never returns when using a StreamingResponse with a client like httpx directly connected to the ASGI app. This is because thelisten_for_disconnectmethod will loop forever receiving the same message like:...without control ever going back to the event loop.
I'm not sure of a better solution to this as it seems like this is necessary when working with a combination of a
while Trueloop andasyncio.wait().